Adding tests for syncing service endpoint and endpointslice to host #3187
Conversation
sowmyav27
force-pushed
the
add-test-eng-6661
branch
2 times, most recently
from
8f1fb9a to
dbc7868
Compare
sowmyav27
force-pushed
the
add-test-eng-6661
branch
3 times, most recently
from
cd0c636 to
49483fb
Compare
|
|
||
| translatedServiceName := translate.SingleNamespaceHostName(serviceName, serviceNamespace, translate.VClusterName) | ||
|
|
||
| ginkgo.By("Verify Service exists in Host Cluster") |
There was a problem hiding this comment.
I noticed this is only a check about existence. But is this sufficient? I'd test also the content of the objects. E.g. the presence of the input values from above:
Ports: []corev1.ServicePort{
{
Name: "custom-port",
Port: 8080,
Protocol: corev1.ProtocolTCP,
TargetPort: intstr.FromInt(5000),
},
},
This also applies to the endpoint object.
There was a problem hiding this comment.
This test wasn't specifically to test it, as the other test was checking the details. But i have added the resource checks also.
test/e2e/servicesync/servicesync.go
Outdated
|
|
||
| ginkgo.Context("Verify endpoint sync when endpoint is deployed before service", func() { | ||
| ginkgo.It("Should sync Service, Endpoints, and EndpointSlice from vCluster to host cluster", func() { | ||
| ctx := f.Context |
There was a problem hiding this comment.
nit: usage of ctx vs. f.Context is not consistent. The object creations use .Create(f.Context, ... and all other use .Get(ctx, ...
|
|
||
| ginkgo.By("Create Service Endpoint in vCluster") | ||
| _, err := f.VClusterClient.CoreV1().Endpoints(serviceNamespace).Create(f.Context, testEndpoint, metav1.CreateOptions{}) | ||
| framework.ExpectNoError(err) |
There was a problem hiding this comment.
I'd always prefer gomega.Expect(err).To(gomega.Succeed()) over framework.ExpectNoError(err)` because:
- it delivers way more context in case of failure. E.g. the whole nested error chain instead of only the aggregated error message.
- it is universal, can be used in any test, not only the once using
framework.
But this is personal preference, feel free to just close this comment in case of disagreement.
There was a problem hiding this comment.
Looks like framework.ExpectNoError is a wrapper for gomega.Expect(err).. So keeping that as is.
sowmyav27
force-pushed
the
add-test-eng-6661
branch
from
49483fb to
c9c73c7
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
sowmyav27
force-pushed
the
add-test-eng-6661
branch
3 times, most recently
from
6a0b3b6 to
4759fa8
Compare
sowmyav27
force-pushed
the
add-test-eng-6661
branch
from
4759fa8 to
db22aaf
Compare
sowmyav27
force-pushed
the
add-test-eng-6661
branch
from
db22aaf to
462a60c
Compare
sowmyav27
force-pushed
the
add-test-eng-6661
branch
from
462a60c to
bbd90b9
Compare
2 participants
What issue type does this pull request address? (keep at least one, remove the others)
/kind test
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>if possible)For eng-8321
Note
Adds ordered e2e coverage to validate vCluster→host sync of Service, Endpoints, and EndpointSlice when Endpoints are created before the Service, with setup/cleanup and assertions across both clusters.
test/e2e/servicesync/servicesync.go:Endpointsexist beforeServicefor a headless service.Service,Endpoints, andEndpointSlicein both vCluster and host (labels, ports8080→5000, IP1.1.1.1).translate.SingleNamespaceHostNameandkubernetes.io/service-nameselector.ginkgo.Orderedand describe updated; addBeforeAll/AfterAllto create and cleanup test fixtures.gomega,fmt,intstr) and validation viagomega.Eventually.Written by Cursor Bugbot for commit bbd90b9. This will update automatically on new commits. Configure here.